Skip to content

ci: adopt the Developer Certificate of Origin - #1562

Merged
jmrplens merged 1 commit into
bp-plaintext-listenerfrom
bp-dco
Oct 28, 2026
Merged

jmrplens merged 1 commit into
bp-plaintext-listenerfrom
bp-dco

Conversation

@jmrplens

@jmrplens jmrplens commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Refs #1545
Refs #1269

What this changes

The project now adopts the Developer Certificate of Origin 1.1. Every commit of a pull request has to carry a Signed-off-by: trailer naming its author, which is how the author certifies the certificate for that commit. This is the OpenSSF Best Practices criterion dco, which until now could only be answered as not met.

I added cmd/audit_dco, which judges the commits reachable from -head and not from -base. It reads each commit from git with the trailers git finds in its message, through git log's %(trailers) placeholder, so a sign-off counts exactly when git sees one. git reads the message's last paragraph as trailers when every line of it is one, or when at least a quarter of its lines are and one of them is a trailer git writes itself, as Signed-off-by is: a Signed-off-by line outside that paragraph is not a trailer, one sharing the paragraph with a line of prose is, and so is a key in lower case. A commit passes when one of its trailers names its author, the name and the email compared without regard to case. It fails with no trailer, or when its trailers name only other people or nobody.

git log prints each commit's fields between separator bytes, and a message can carry them. A record ends only where its separator is followed by a newline, which no field git prints holds, so such a message is read as git reads it: the byte stays in its field, and a trailer it ends names nobody. The commits are also listed by git rev-list, and a run where the two lists differ stops, so a sign-off can never move onto another commit. Text a commit's author chose is printed with its control characters replaced, so a subject cannot start a workflow command of its own.

A merge of two parents that brings no change of its own is reported and not judged. git show --remerge-diff prints nothing for it, since its tree is the one git's own merge of its parents makes, and that is the merge GitHub's Update branch button writes. Every other merge is judged like any other commit: one that resolves a conflict, adds or drops a change while merging, or joins more than two parents, which git does not remerge. Without that, a pull request could carry its whole change in an unsigned merge and pass.

A declared bot's commits are exempt in a pull request that bot opened, and only there. Dependabot is the one declaration in bots.go, with the identity its commits carry and the login that opens its pull requests, and a commit claiming that identity in a pull request somebody else opened is judged like any other. The release workflow's version stamp is pushed to main and never sits in a pull request, so nothing judges it and it needs no declaration.

I decided the description is not judged. GitHub copies the Signed-off-by trailers of a pull request's commits into the squash commit, below the description and once each: the squash commits of #1512 and #1529, whose descriptions carry no trailer, carry the maintainer's, and Dependabot's squash commits carry Dependabot's. Asking the description for its own trailer would only repeat it.

make check-dco runs the command on origin/main..HEAD by default. ci.yml gains a ✍️ DCO sign-off job that runs it on every pull request, with the event's base and head commits, the login that opened it and annotations. The event of a stacked pull request names the head of the layer below as its base: each of #1519 to #1525 had the head of the layer below as its base.sha, and the em dash job of #1519 read 4b110403, the head of issue-958a-zero-and-capacity. So each layer's pull request judges that layer's commits and no others. The comment on the em dash job said its base was the stack's base, and I corrected it. The job skips itself on a push to main, and the checks job waits on it and counts that skip as a pass, as it already does for the image build.

Documentation

CONTRIBUTING.md gains a Developer Certificate of Origin section: what the sign-off certifies, how to make it with git commit -s, how to sign off commits already made with git rebase --keep-base --signoff origin/main, what the check judges and exempts (a merge that changes something is concluded with git commit -s), and why the squash commit needs nothing more. The Before Submitting checklist, the review standard and the acceptance list name it, and the pull request template gains a checkbox.

GOVERNANCE.md lists the sign-off among a contributor's responsibilities. The site's about page says it in English and Spanish, linking the CONTRIBUTING section.

The repository settings page gains a Web commit sign-off section. That setting makes GitHub sign off the commits it makes in its web interface, such as an applied review suggestion, and it is off today: the page says so, with the date I read it, and says that until it is on such a commit needs the trailer typed into its message.

The git-commit skill, which CLAUDE.md sends commits through, now commits with -s and says why.

The command reference, the repository map, CLAUDE.md (the rule, the tree, and the package count, 309 in the module and 336 directories holding tracked Go files) and the Copilot instructions name the command. The testing reference is regenerated once, at the top of the stack.

Tests

I wrote the tests first. Against stubs of every function, with ci.yml and the Makefile as they stood before this layer, 39 of the 40 test functions fail, 118 tests and subtests in all; the one that passes reads an empty log, whose answer is the stub's. They now pass and cover every statement of the package, all 104 conditions and all 80 branches by gobco, and gremlins kills all 58 mutants.

The cases run over real repositories built in temporary directories: commits signed off by their author, by somebody else, by nobody and by git commit --signoff itself; a lower-case key; a Signed-off-by line outside the last paragraph and one sharing it with prose; a merge that brings no change, one that adds a file while merging, one made with --strategy=ours and one of three parents; a merge whose tree the clone lacks; Dependabot's commit in its own pull request and in a person's; the separator byte in a subject, inside a trailer value and at the end of the last trailer; and ranges git cannot read. Three contract tests hold ci.yml (two) and the Makefile (one) to the command line the job relies on.

I also ran the command on this repository's history. From 8f339f4e8, the stack's base on main, to this layer's head, all 50 commits pass: the layers below were written before the adoption, and rebasing the stack onto that main with git rebase --signoff signed each of their commits off. Nothing in CI judges the layers below this one, which run a ci.yml without the job, so a commit added to one of them later needs git commit -s all the same. Dependabot's squash commit 065b75b15 is exempt with -opened-by 'dependabot[bot]'.

What only the maintainer can do

  • fix(tools): handle paginated merge request diffs #1517. It is the one other pull request open on 27 October. An outside contributor opened it, its six commits carry no sign-off, and only its author can add one. The first CI run a push to it starts after this merges includes the job, which fails. The ways through are merging fix(tools): handle paginated merge request diffs #1517 before this, asking its author to sign off (a comment on their pull request, which waits for your yes), or limiting the job to pull requests opened after the adoption, which the decision to adopt the certificate allows.
  • Web commit sign-off. It is a repository setting: gh api -X PATCH repos/jmrplens/gitlab-mcp-server -F web_commit_signoff_required=true. Until it is on, a commit made in GitHub's web interface needs the trailer typed by hand. Once it is on, the Current state paragraph of the Web commit sign-off section in docs/development/repository-settings.md and the sentence in CONTRIBUTING that says the setting is off both change.
  • The acceptance test. The plan asks for a test pull request whose unsigned commit fails the job and whose signed commit passes. This pull request's own run shows the signed half; the unsigned half needs a test pull request, which I have not opened.
  • The issue. This pull request refers to Adopt the Developer Certificate of Origin #1545, the issue of this layer, rather than closing it: its work is done once web commit sign-off is on and a test pull request has shown the job failing, and I close it then by hand.

Checks run

  • go test ./cmd/audit_dco/ -count=1 -cover: 100.0% of statements.
  • make coverage-conditions PKG=./cmd/audit_dco, in condition mode (104/104) and branch mode (80/80).
  • make coverage-mutants PKG=./cmd/audit_dco: 58 killed, 0 lived, 0 not covered.
  • golangci-lint run --build-tags e2e ./cmd/audit_dco/ and golangci-lint fmt --diff: clean.
  • make check-supply-chain, check-godocs, check-doc-tool-names, check-action-ids, check-test-goroutines, check-test-subtests, check-test-file-names, check-dead-consts, check-md-tables and check-tenancy: all pass.
  • GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred go test ./... -count=1: all pass.
  • make check-dco DCO_BASE=bp-plaintext-listener: this layer's commit passes.
  • scripts/check-em-dash.sh diff bp-plaintext-listener and make check-pr-description on this text: pass.
  • markdownlint on every changed Markdown file, node scripts/check-doc-links.mjs, and the site's i18n:check, facts:check, chips:check, llms:check, version:check, build and lint: all pass.

On the rebased stack

I rebased the stack onto main at 8f339f4e8 on 27 October 2026. The checks above ran on this layer before that rebase. On the rebased head, go build ./..., go vet ./... and go test ./... -count=1 pass with the freshness comparisons deferred, as CI runs every layer below the top, and make check-dco passes over every commit from that main to the top of the stack. The committed generated artifacts are regenerated once at the top of the stack, which runs every gate over the combined tree.

After the first CI round

SonarCloud found runGit running git by its name (S4036) and four test variables in cmd/audit_dco/report_test.go that only fed the condition below them (godre:S8193). runGit now resolves git with exec.LookPath and runs the absolute path, as cmd/gen_orbit_record and cmd/gen_model_results already do, and a PATH holding no git is reported as the lookup's failure, which a new test holds; the four conditions read the call directly. Both are folded into the layer's commit. cmd/audit_dco stays at 100.0% of statements, 106 of 106 conditions and 82 of 82 branches, golangci-lint is clean, and gremlins kills the 22 mutants of git.go with none lived or uncovered.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 2fc08a8 and 6d6f835.

Files selected for processing: 24
  • .github/copilot-instructions.md
  • .github/pull_request_template.md
  • .github/skills/git-commit/SKILL.md
  • .github/workflows/ci.yml
  • CLAUDE.md
  • CONTRIBUTING.md
  • GOVERNANCE.md
  • Makefile
  • cmd/audit_dco/bots.go
  • cmd/audit_dco/bots_test.go
  • cmd/audit_dco/doc.go
  • cmd/audit_dco/git.go
  • cmd/audit_dco/git_test.go
  • cmd/audit_dco/main.go
  • cmd/audit_dco/main_test.go
  • cmd/audit_dco/report.go
  • cmd/audit_dco/report_test.go
  • cmd/audit_dco/signoff.go
  • cmd/audit_dco/signoff_test.go
  • docs/development/cmd-utilities.md
  • docs/development/repository-map.md
  • docs/development/repository-settings.md
  • site/src/content/docs/about.mdx
  • site/src/content/docs/es/about.mdx
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jmrplens
jmrplens added this pull request to stack #1565 October 27, 2026 03:07

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 3 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="cmd/audit_dco/report.go" line_range="62-85" />
<code_context>
+		_, _ = fmt.Fprintf(w, "%s: no commits in %s, so nothing to judge\n", toolName, r.span)
</code_context>
<issue_to_address>
**Output errors are ignored**

When a supplied output writer returns an error, such as stdout being unable to write the report, `report.write` discards every `fmt.Fprintf` error, so a failed report write can still make `runMain` return 0 and present a passing check without its promised report. `runMain` also discards errors writing diagnostics.

Propagate write errors from the report and command, and return a non-zero status when output cannot be written.

Also at `cmd/audit_dco/main.go:40-65`.
</issue_to_address>

### Comment 2
<location path="cmd/audit_dco/signoff.go" line_range="18-21" />
<code_context>
+// never be an author's), and the email holds an at sign and no angle bracket
+// or blank.
+func parseIdentity(value string) (identity, bool) {
+	name, rest, _ := strings.Cut(strings.TrimSpace(value), "<")
+	email, closed := strings.CutSuffix(rest, ">")
+	name, email = strings.TrimSpace(name), strings.TrimSpace(email)
+	if !closed || name == "" || strings.ContainsRune(name, '>') ||
+		!strings.Contains(email, "@") || strings.ContainsAny(email, "<> \t") {
+		return identity{}, false
</code_context>
<issue_to_address>
**Valid author names fail DCO check**

When an author’s Git identity contains `>` in the name, `parseIdentity` splits at the first `<` and rejects `>` in the name, so a valid author name containing `>` makes the matching `git commit -s` trailer unparsable. `judge` then fails that commit, and the pull request’s DCO check fails despite the author’s sign-off.

Parse sign-off identities using Git-compatible identity boundaries rather than rejecting bracket characters that can occur in a valid author name.
</issue_to_address>

### Comment 3
<location path=".github/workflows/ci.yml" line_range="1203" />
<code_context>
+  # commits, and the release workflow's version stamp, which never sits in a
+  # pull request.
+  dco:
+    name: "✍️ DCO sign-off"
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
</code_context>
<issue_to_address>
**DCO guidance contains emoji**

The new workflow label and contributor guidance use the ✍️ emoji, contrary to the repository's stated convention.

Remove the emoji from the job name and the documentation references.

Also at `CONTRIBUTING.md:152`, `docs/development/repository-settings.md:559`, `docs/development/repository-settings.md:580`.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 2 findings to address first, and the new CI gate requires author sign-offs, and GitHub copies those DCO attestations into public squash-commit history when pull requests merge. Reverting disables the gate, but it does not remove attestations already published in that history.

Blocking findings: cmd/audit_dco/report.go:85, cmd/audit_dco/signoff.go:21


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread cmd/audit_dco/report.go Resolved
Comment thread cmd/audit_dco/signoff.go Resolved
Comment thread .github/workflows/ci.yml Resolved
@github-actions github-actions Bot added ci Build pipeline, workflows, or lint configuration tooling The audit and generator commands under cmd/, and the Makefile targets that run them labels Oct 27, 2026
@jmrplens jmrplens added this to the 3.4.0 milestone Oct 27, 2026
The project now asks every commit of a pull request to carry a
Signed-off-by trailer naming its author, which is how the author
certifies the Developer Certificate of Origin 1.1 for it. This is the
OpenSSF Best Practices criterion dco, which until now could only be
answered as not met.

cmd/audit_dco judges the commits reachable from -head and not from
-base. It reads each one from git with the trailers git finds in its
message, through git log's %(trailers) placeholder, so a sign-off counts
exactly when git sees one. A commit passes when one of its Signed-off-by
trailers names its author, name and email compared without regard to
case. It fails with no trailer, or with trailers that name only other
people or nobody. A record of git log's output ends only where its
separator byte is followed by a newline, which no field git prints
holds, so a message carrying the separator is read as git reads it; git
log is also held to the hashes git rev-list lists for the same range, so
a sign-off can never move onto another commit. Text an author chose is
printed with its control characters replaced, so a subject cannot start
a workflow command of its own.

A merge of two parents that brings no change of its own, which git show
--remerge-diff prints nothing for and which is what GitHub's Update
branch button writes, is reported and not judged. Every other merge is
judged like any other commit: one that resolves a conflict, adds or
drops a change while merging, or joins more than two parents. A declared
bot's commits are exempt in a pull request that bot opened, and only
there: Dependabot is the one declaration, with the identity its commits
carry and the login that opens its pull requests, and a commit claiming
that identity in a pull request somebody else opened is judged like any
other. The release workflow's version stamp is pushed to main and never
sits in a pull request, so nothing judges it.

The description is not judged. GitHub copies the Signed-off-by trailers
of a pull request's commits into the squash commit, below the
description and once each, as the squash commits of #1512, #1529 and
Dependabot's pull requests show, so main's history carries them without
a trailer in the description.

make check-dco runs it on origin/main..HEAD by default. ci.yml gains a
dco job that runs it on every pull request with the event's base and
head commits, the login that opened it and annotations, and skips itself
on a push to main; the checks job waits on it and counts that skip as a
pass, as it does for the image build. The event of a stacked pull
request names the head of the layer below as its base, so each layer's
pull request judges that layer's commits and no others, which the em
dash job's comment now says too.

CONTRIBUTING.md gains a Developer Certificate of Origin section, with a
checklist item and the review standard's mention, and the pull request
template a checkbox. GOVERNANCE.md lists the sign-off among a
contributor's responsibilities, and the site's about page says it in
English and Spanish. The repository settings page records web commit
sign-off, which GitHub applies to commits made in its web interface such
as an applied suggestion and which is off today. The git-commit skill
commits with -s. The command reference, the repository map, CLAUDE.md,
with the rule and the package counted (309 in the module, 336
directories holding tracked Go files), and the Copilot instructions name
it; the testing reference that counts its tests is regenerated once, at
the top of the stack.

The tests were written first. Against stubs of every function, with
ci.yml and the Makefile as they stood before this commit, 39 of the 40
test functions fail, 118 tests and subtests in all; the one that passes
is the empty log's, whose answer is the stub's. They now cover every
statement, all 104 conditions and all 80 branches, and gremlins kills
all 58 mutants.

Refs #1269

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
@sonarqubecloud

Copy link
Copy Markdown

@jmrplens

Copy link
Copy Markdown
Owner Author

On the review summary's note that the sign-offs end up in main's squash commits and that reverting the gate would not remove them: that is the intent. GitHub appends the pull request's Signed-off-by trailers to the squash commit, as main already shows (8f339f4, 6049193). Clause (d) of the certificate, summarized in CONTRIBUTING.md, is precisely that the contribution and its sign-off are public and kept. Adopting the DCO was a deliberate decision, so no change is needed.
For future reviews of this repo: a sign-off recorded permanently in main's history is the purpose of the DCO check, not a risk to flag.

@jmrplens
jmrplens merged commit 3da05d1 into main Oct 28, 2026
38 of 67 checks passed
@jmrplens
jmrplens deleted the bp-dco branch October 28, 2026 09:38
jmrplens added a commit that referenced this pull request Oct 28, 2026
…ndency and regenerate the testing reference (#1564)

Refs #1269, #1114, #1243, #1129

This is the top of the stack that carries the OpenSSF Best Practices
work of #1269 and the supply-chain layers below it (#1114, #1243,
#1269's Scorecard workflow and #1129's image scan). The layers below
leave the committed generated artifacts stale under the freshness
deferral, and this one regenerates them once, on the tree the merge puts
on `main`. It also carries the fixes the review bots' threads on the
layers asked for, each in a commit of its own (Review fixes, below); it
changes no handler.

The whole stack sits on `main` at `8f339f4e8`, onto which I rebased it
on 27 October 2026, layer by layer and in order. Every commit of it is
signed and carries a `Signed-off-by` trailer: the layers written before
the maintainer adopted the Developer Certificate of Origin got theirs
from `git rebase --signoff`. Where a layer met `main`'s configuration
stack (#1518 to #1525) or the Orbit fixtures' move (#1532) the conflicts
kept both sides, and generated files took `main`'s side to be
regenerated here. The supply-chain stack's own regeneration,
`c-c1-regen`, conflicted with `main`'s testing reference, held nothing
else, and was dropped, so `issue-1129-trivy` is now the top of that
part. The header rewrite of `bp-file-headers` was made again by `make
fix-file-headers` on the rebased tree rather than rebased.

## What it regenerates

Only the testing reference moves, and the action record gains the one
file a review fix reads. The layers and the review fixes add 271 test
functions, 266 under `cmd/` and five in `test/e2e/http`, 41 test files
under `cmd/` and seven packages, so
`docs/development/testing/testing.md` gains them in its totals, its
naming breakdown, its distribution table and the `cmd` coverage table. I
refreshed the counts with `go run ./cmd/gen_testing_docs/
-skip-coverage`, which carries every recorded coverage figure forward
and writes `n/a` for a package no coverage run has measured. The six new
packages that have statements (`cmd/audit_dco`,
`cmd/audit_file_headers`, `cmd/audit_image_vulns`,
`cmd/audit_site_headers`, `cmd/fuzz_targets` and
`cmd/reproduce_release`) carry the 100.0% `go test -covermode=count
-cover` reports for each, and `cmd/internal/fileheader`, which holds
constants alone, keeps `n/a`. A second `-skip-coverage` run keeps every
figure.

Every other generator of `make update-all` runs on this tree and writes
nothing: the brand assets, the action grants, the token footprint, the
site stats, the llms files, the per-domain tool reference in both
languages, the LobeHub manifest, the model corpus ledger and results
page, the action catalog manifest, the benchmark charts and the e2e
coverage record page, and the table formatter after them. The request
inventory, recorded again over the whole unit suite on this tree, does
not move (1680 rows, 976 paths, 1090 of 1090 catalog actions owned by a
package that recorded a request). `go run ./cmd/audit_supply_chain
-record` reads the 32 pinned actions again from GitHub and writes only
the Dockerfile of `hadolint/hadolint-action`, which the review fix for
#1552 reads. The package and file counts in `CLAUDE.md` match the tree:
311 packages in the module, 338 directories holding tracked Go files,
182 under `internal/tools`, 788 non-test and 488 test Go files there.

## Checks on this head

With the freshness comparisons checked
(`GITLAB_MCP_TEST_SNAPSHOT_PARITY` unset):

- `go test ./... -count=1`: 301 packages pass, recording the request
inventory as they run, and `go run ./cmd/gen_request_inventory/ -check`
passes after them (1680 rows, 976 paths). The coverage job's own command
passed with a total of 100.0% of statements on the head before the
review fixes, and `cmd/audit_supply_chain`, the one package whose code
they change, is at 100.0% of statements, 1532 of 1532 conditions and
1118 of 1118 branches under gobco, with gremlins leaving no mutant of
the five files they change lived, uncovered or timed out.
- Every `make` target the generated artifacts job runs, 45 of them with
`check-closed-servers`, `check-supply-chain`, `check-e2e-static` and
`audit-catalog-first`, passes; `check-openplugin`,
`check-server-json-packages` (the eight declared packages),
`check-file-headers` (3434 source files) and `check-dco` (all 73 commits
from `8f339f4e8`) pass locally, since the first two need `jq` and the
network and the last two read git.
- `make analyze` on the remote runner: 30 of 32 steps pass. Step 6,
markdownlint, has no `npx` there, and step 32, the file headers, has no
`.git`; both pass locally, markdownlint over `"**/*.{md,mdx}"`.
- `make test-e2e-http` (780 tests, the 8 that need Docker for a proxy or
a balancer skipped) and `make test-e2e-stdio` (277 tests) pass.
- The server binary and the e2e suite's compiles of the compile job, and
the type-check job's vets for windows/amd64, darwin/arm64 and
linux/arm64, pass.
- Locally: the repository scripts' 517 Python tests, `make
check-em-dash` against `main`, `make check-plan-untracked`, `make
check-mcpb`, the site's `i18n:check`, `facts:check`, `chips:check`,
`llms:check`, `version:check`, `build` and `lint`, and `node
scripts/check-doc-links.mjs` (350 files, 8267 anchors) pass.
- Every layer's head below this one passes `go build ./...`, `go vet
./...` and `go test ./... -count=1` with the freshness comparisons
deferred, as CI runs them.
- `make check-pr-description` passes on this text and on every layer's.

## The release rehearsals

Three layers change what `release.yml` does, and each is rehearsed from
its own branch once the stack is pushed: `issue-1129-trivy` for the
supply-chain layers (the PyPI publish and the image scan),
`bp-release-notes` for the written summary preflight requires, and
`bp-reproducible-builds` for the rebuild compared byte for byte. `gh
workflow run release.yml --ref <branch>` builds, smoke-tests and
dry-runs every publisher from that branch and publishes nothing. Its
race gate runs the unit suite without the deferral, and the packages
whose tests compare a committed artifact pass that way on each of the
three branches and on `bp-image-base`. The review fix to
`scripts/publish-pypi.sh`, whose `--dry-run` the rehearsal's PyPI job
runs, lives on this branch, so the PyPI half of the `issue-1129-trivy`
rehearsal no longer exercises the script `main` will carry, and
`bp-regen` is rehearsed as well for it.

## After the first CI round

The first CI round of the stack asked for fixes in ten layers, each made
in the layer that owns it and folded into the commit it belongs to, or
added as a commit of that layer where it is new work, and every layer
above rebased on it:

- SonarCloud: the duplicated literals of #1550, #1552 and #1557, the
nested anonymous structs of #1553, the eight-parameter functions of
#1558, the tool run by its name from PATH in #1557, #1558, #1562 and
#1563 (each now resolved with `exec.LookPath`, as `cmd/gen_orbit_record`
does), and the test variables of #1562.
- GitGuardian on #1551: the stand-in OIDC token of the PyPI publish
tests is built at run time, and no commit of that layer carries the
literal.
- `windows-latest` on #1555: a file in the release notes directory's
place is refused on every system.
- The whole-tree condition measurement of #1558, which could not measure
31 packages: `scripts/coverage-conditions.sh` stages the two external
test package shapes gobco v1.3.4 cannot read, two tests read the same
thing in gobco's copy as in the source, and the 31 are measured and
recorded. I decided on 27 October to keep that staging, which supersedes
my decision of 6 October to leave the limit to gobco, until gobco
publishes a release with the fixes; upstream-bugs entries 97 and 140,
the script and the testing page say it is temporary.
- #1561 gained the hardened mode the maintainer decided on:
`--allow-plaintext-http` defaults to `true`, and `false` refuses a
plaintext listener off loopback, as register row ADM-015.

This head regenerates the testing reference once more for the tests that
round added. On every layer's head, `go build ./...`, `go vet ./...` and
`go test ./... -count=1` pass with the freshness comparisons deferred,
and every package whose code that round changed is at 100.0% of
statements, with every condition and branch gobco counts there taken and
no mutant lived or uncovered beyond the one constant initializer of
`cmd/server/listen.go` the layer below names.

## Review fixes

The review bots' threads on #1550 to #1562 found ten real gaps, and I
fixed each here, at the top, in a commit of its own, so no layer below
changes and only this pull request runs CI again; every thread, declined
ones included, is answered with the commit or with why it does not
apply. In `cmd/audit_supply_chain` (#1550, #1552, #1555): a GoReleaser
template is a value only the run knows and a build's overrides and a
flags string are read, an absolute `--config`, `dir` or `dist` stays
absolute and is refused, a tag-named image is refused in a job whose
token neither it nor its workflow declares, the images a container
action's Dockerfile builds from are held to their digest, read as
BuildKit binds the file's stages, parser directives and heredocs (the
record gains `hadolint/hadolint-action`'s Dockerfile, whose base is
pinned), a release note crediting an assistant as its author is refused,
and release note headings and fences are read as GitHub renders them.
`TestClient_StatelessAnswersCarryNoSession` holds the stateless
initialize to a JSON-RPC result (#1550), `scripts/publish-pypi.sh`
publishes exactly the six platform wheels, each a regular file (#1551),
the install pages say `make check-repeatable` does not build the
universal macOS binary (#1557), and the Code of Conduct offers the
OpenPGP key for a report sent by email (#1560). Each testable fix came
with a test that failed before it, `cmd/audit_supply_chain` stays at
100.0% of statements, and the testing reference is refreshed for the
seven test functions they add.

## Dependencies

This layer also moves every dependency to its newest release, with no
cooldown, which fixes this PR's CI failure: golang.org/x/net v0.62.0
fixes GO-2026-6860 and GO-2026-6866 in x/net/html, and
check-binary-vulns now reports no advisory on the six binaries. The
other Go moves are tiktoken-go/tokenizer v0.10.0 (whose Codec interface
gained two methods that the cmd/audit_tokens test doubles now
implement), gqlparser v2.5.64, grpc v1.85.0, grpc-gateway v2.33.0 and
their indirect dependencies. The builder image moves to the rebuilt
golang:1.27.3-alpine3.24 digest.

Five actions move: setup-python v7.1.0, markdownlint-cli2-action
v25.0.0, login-action v4.7.0, codeql-action v4.39.1 and
sonarqube-scan-action v8.4.0, with pinned-actions.json re-recorded. The
release tools move to GoReleaser v2.19.0, cosign v3.1.4, oras 1.3.6 and
mcp-publisher v1.8.6, the NuGet validation to the .NET 10.0.403 SDK
(both dnx facts the install page relies on re-verified), and the npm
validation and publishing to Node 24. The PyPI upload's lock is
recompiled (platformdirs 4.13.7, pyasn1 0.6.5).

The site moves to pnpm 12.16.1, Node 24.22.1 and the patch releases of
astro, starlight, markdown-remark, @astrojs/check and typescript-eslint.
The telemetry example moves to collector 0.164.0 and otel-lgtm 0.38.1,
the balancer e2e to HAProxy 3.4, and the e2e Bitbucket fixture to 10.5,
which now starts with basic authentication force-allowed: Bitbucket 10
blocks it by default, and both the fixture's setup script and GitLab's
importer authenticate with it.

Left out on purpose: client-go stays at v3.30.0 because it moves in its
own stack (the server's major tracks it, and a bump moves the served
surface and the fine-grained table); the GitLab CE/EE images stay pinned
because 19.5 is still GitLab's newest release and the fine-grained table
records it; Node 26 is not taken because no 26.x is LTS and it drops
Corepack; TypeScript 7 is refused by the peer ranges of @astrojs/check
and typescript-eslint. The release tools, the SDK and the builder image
were rehearsed with `release.yml` on this branch before merging.

## Before this merges

- **#1517.** It is the outside contribution open on 27 October, based on
`main` at `e3e09d290`. Its 18 changed source files merge with the top of
this stack without a conflict; the file it adds has no header and its
six commits no sign-off, which the header and DCO layers say how to
handle.

Milestone: 3.4.0.

---------

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
jmrplens added a commit that referenced this pull request Oct 28, 2026
…uires it (#1566)

I turned on **Require contributors to sign off on web-based commits**
today, after the DCO check of #1562 merged, so GitHub now signs off a
commit made in its web interface, such as an applied review suggestion,
with its author's identity.

This updates the two places that still said the setting was off: the
sentence in `CONTRIBUTING.md` that asked for a trailer typed by hand,
and the **Current state** of the web commit sign-off item in
`docs/development/repository-settings.md`, which now reads applied, with
`web_commit_signoff_required: true`.

Documentation only; markdownlint and `node scripts/check-doc-links.mjs`
pass.

Signed-off-by: José M. Requena Plens <mail@jmrp.io>
jmrplens added a commit that referenced this pull request Oct 29, 2026
…'s diff, publish the pagination of mr_review.changes_get, and say when a prompt read only the first page of changed files (#1584)

An inline discussion or draft note on a file past the first page of a merge request's diff was refused locally as "not in the merge request diff", because the position pre-check read one page of 100 files, and `mr_review.changes_get` returned one page without saying how to continue. Nikita Turchaninov (slowaner) found and fixed both in #1517, and that work is his: the pre-check walks the diff a page at a time and stops at the file, bounded by the new register row RQB-013; it compares the merge request's `diff_refs` with the position before and after the walk; and it leaves the write to GitLab whenever its evidence is incomplete. `changes_get` publishes its pagination, and its Files line reads `N`, `N of total` or `N shown, more available`.

This pull request carries #1517 to land it for 3.4.0, with him as co-author, and I'll close #1517 when it merges. His six commits carry no `Signed-off-by:` trailer, which the Developer Certificate of Origin needs on every commit since #1562, and adding one means rewriting his branch, so I squashed them into one commit of mine on today's `main`, `fix(tools): handle paginated merge request diffs`, with a `Co-authored-by:` trailer naming him, and opened it from a branch of this repository. The merge conflicted only in generated files (the README token block, the testing reference and the token footprint), and I regenerated them from the merged tree rather than taking a side. The branch is rebased on the localization stack (#1577 to #1582), where only the testing reference conflicted, regenerated the same way. #1530 and #1531 stay separate, as agreed in the review of #1517.

## What I added on top

- `internal/tools/mrchanges/action_specs_test.go`, the one file the pull request adds, opens with the copyright and license header #1563 requires, written by `make fix-file-headers`.
- The doc comment of `toolutil.MergeRequestDiffForPositionCheck` says again why skipping the check is safe: the write goes to the same instance with the same credential, so a 401, 403 or 404 on the listing or on the snapshot read is the refusal the write is about to get, and a transient failure, a snapshot that moved or an exhausted page budget leaves the write judged by GitLab alone. Both `validatePosition` comments that send the reader there are true again, and name the snapshot case.
- The same gap in the prompts. The five prompts that read a merge request's changed files (`summarize_mr_changes`, `review_mr`, `suggest_mr_reviewers`, `mr_risk_assessment` and `mr_description_quality`) read one page of 100, as their descriptions have said since issue 1169, but counted it as the whole diff: a review or a risk assessment of a merge request changing 250 files read "Files changed: 100", with the line totals of those 100 and nothing saying the other 150 were never read. Their count now reads as `changes_get`'s does, and when the merge request changes more files a line says that every count and list covers the page alone and that `mr_review.changes_get` reads the rest from page 2 with `per_page` 100. A merge request within one page gets exactly the message it got before. `TestMRDiffPrompts_MoreFilesThanOnePage_SayWhatTheyRead` fails on the parent in every partial case, and the prompt section of the site says the same in English and Spanish. The line stays English under `GITLAB_MCP_LANGUAGE=es`, like the rest of a `prompts/get` body, since the model reads it and ADR-0027 keeps what the model reads out of the message catalog.
- `suggest_mr_reviewers` reads the merge request and its changed files as `review_mr` does, but its exclusion entry named only `project.members`, so an operator who excluded `mr_review.changes_get` or `merge_request.get` was still served the changed paths through it, and with the line above it would have named the excluded action. It now names both, and `TestRegister_ExcludingAnActionRemovesThePromptServingTheSameData` fails without that.

## The other diff routes

I went through every handler that reads a diff for the same gap. Whether GitLab pages a route is what the live record says it takes: only the merge request diffs, the diff versions and the commit diff declare `page` and `per_page`.

| Route | Read by | Paged | Affected |
| --- | --- | --- | --- |
| `GET /projects/:id/merge_requests/:merge_request_iid/diffs` | `mr_review.changes_get` and the position pre-check of `mr_review.discussion_create`, `mr_review.draft_note_create` and `mr_review.draft_note_update` | Yes | Yes, fixed by slowaner's commit |
| The same route | the five prompts above | Yes | Yes, fixed here: still one page, now said in the message |
| `GET /projects/:id/merge_requests/:merge_request_iid/versions` | `mr_review.diff_versions_list` | Yes | No: it takes `page` and `per_page` and publishes its pagination |
| `GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id` | `mr_review.diff_version_get` | No | No |
| `GET /projects/:id/merge_requests/:merge_request_iid/raw_diffs` | `mr_review.raw_diffs` | No | No |
| `GET /projects/:id/repository/commits/:sha/diff` | `repository.commit_diff` | Yes | No: it takes `page` and `per_page` and publishes its pagination |
| `GET /projects/:id/repository/compare` | `repository.compare` and the `generate_release_notes`, `compare_branches` and `audit_commit_hygiene` prompts | No | No |
| `GET /projects/:id/merge_requests/:merge_request_iid/changes` | nothing | No | No |

## How I tested it

On the head, `go test ./... -count=1` passes (303 packages), recording the request inventory on the whole unit suite, and `gen_request_inventory -check` finds the committed inventory current. `golangci-lint` passes on every package the branch touches. `audit-1to1-paths`, `audit-1to1-grants`, `check-action-grants`, `check-action-grants-derivation`, `check-tenancy`, `check-surface-quality`, `check-tool-reference`, `check-meta-descriptions`, `check-action-ids`, `check-sdk-context`, `check-md-escaping`, `check-i18n`, `check-test-file-names`, `check-test-subtests`, `check-test-goroutines`, `check-godocs`, `check-llms`, `check-testing-docs`, `check-footprint`, `check-site-stats`, `check-md-tables`, `check-doc-tool-names`, `check-e2e-static`, `check-action-catalog-manifest`, `audit-catalog-first`, `check-file-headers`, `check-dco` and `check-em-dash` pass, and the three `test/e2e/gitlab` packages compile and vet under the `e2e` tag. Every generator of `make update-all` runs clean on the head, the testing reference with `-skip-coverage`, which its own commit refreshed. The site passes `i18n:check`, `facts:check`, `chips:check` and `llms:check`, and `node scripts/check-doc-links.mjs` passes. I did not run the Docker suites against a GitLab.

<details>
<summary>Design notes from slowaner's description</summary>

The position pre-check searches for the requested old or new path one page at a time and stops at the first match. It retains no earlier pages. Discussion create and draft-note create and update share this lookup and preserve the requested body, position and SHAs.

Before trusting the listing, the helper reads the merge request's `diff_refs` and compares all three SHAs with the position. It reads them again after finding the file or finishing the listing. Both snapshot reads are kept: the second detects a push during the search, so evidence from a newer diff cannot justify a local refusal. A successful lookup costs k+2 reads for a match on page k, plus the write. This remains best effort, since GitLab validates changes after the final read. A changed, missing or unreadable snapshot, a listing error, cancellation, non-advancing pagination or an exhausted page budget makes the pre-check advisory, and GitLab judges the original write. Only a complete, stable result can produce the existing file or line error. A matching file with no available patch skips line validation.

The walk is bounded by RQB-013 at 100 actual list calls of 100 files. A match or completed listing on call 100 is valid; call 101 is never sent. Reaching the bound adds no caller quota and does not reject the write. GitLab documents a maximum `diff_max_files` of 3,000, normally at most 30 pages at 100 files per page; the 100-call bound also ends a malformed walk that keeps advertising another page. GitLab's own diff-size limits still apply.

| Item | Answer |
| --- | --- |
| VAL-001 | Allow: bounds the diff pages one advisory position check may read. |
| VAL-002 | `KeyRequest`; mint cost is nothing. Each invocation has its own finite walk; this is not an allowance accumulated by a caller. |
| VAL-003 | Class P, as computed for a request bound by the register. No class D finding. |
| VAL-004 | `RequestBound`, not a share or fairness guarantee. |
| VAL-005 | No claim to protect a process-wide resource or one population from another, so no process partner applies. |
| VAL-006 | At 100 list calls, a match or terminal page is accepted. If another page is needed, discard incomplete evidence and let GitLab judge the unchanged write. No table is keyed on caller input. |
| VAL-007 | No new refusal, channel, code, HTTP status, `Retry-After` or retry class, on HTTP or STDIO. Existing stable-snapshot position errors remain unchanged. |
| VAL-008 | Counts actual diff-list calls within this lookup only. It does not charge an admission, tenant or server quota, and charges no unjudged write. |
| VAL-009 | Fixed source constant `mergeRequestDiffPositionMaxPages = 100`; 100 files per call. No new setting, flag or MCP parameter, so operator-supplied zero and malformed values do not apply. |
| VAL-010 | Debug diagnostics state why the advisory check was skipped. No new refusal metric or caller/address identity label; existing project/MR context identifies the operation. |
| VAL-011 | The same request bound and advisory fallback apply on STDIO. |
| VAL-012 | No population-protection or fairness claim, so fairness measurement is not applicable. Boundary, early-stop and fallback behavior are covered by regression tests. |

`changes_get` publishes pagination metadata in structured output and Markdown. Guidance describes offset continuation, `next_page` as `page`, with `per_page: 100`. The keyset request option #1517 first added, and its synthetic endpoint tests, were removed during its review, and the existing input fields stay as they are for #1530.

</details>

Co-authored-by: Nikita Turchaninov <2010628+slowaner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Build pipeline, workflows, or lint configuration tooling The audit and generator commands under cmd/, and the Makefile targets that run them v3.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant